Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let cond of an empty square matrix return zero #1169

Merged
merged 3 commits into from
Jan 29, 2025
Merged

Let cond of an empty square matrix return zero #1169

merged 3 commits into from
Jan 29, 2025

Conversation

martinholters
Copy link
Member

Transfer of JuliaLang/julia#38372. Summary of the discussion there:
We have this inconsistency:

julia> cond(ones(0,0), 1)
0.0

julia> cond(ones(0,0), 2)
ERROR: ArgumentError: reducing over an empty collection is not allowed; consider supplying `init` to the reducer

julia> cond(ones(0,0), Inf)
0.0

If it wasn't breaking, making all cases throw would be acceptable behavior. This PR goes the other way, making the p==2 case also return zero. Although met with some skepticism, if there is to be chosen a value, zero is probably it.

I think the main question is whether the inconsistency or the somewhat dubious return-value is the lesser evil. Mainly porting this over as a reminder to reach a decision. Either way we should probably close #778; either by resolving it with this PR or as won't-fix.

Fixes #778.

@stevengj
Copy link
Member

stevengj commented Jan 14, 2025

Thinking about this a bit more, this seems sensible, at least for square empty matrices. For square matrices, we define the condition number as $\Vert A \Vert \cdot \Vert A^{-1} \Vert$. For an empty matrix, the norm should be zero (because it's the additive identity), and the inverse of a square empty matrix is itself (since $Ax = b$ has a unique solution: an empty $b$ is unique, and empty $x$ solves it).

Returning 0 is also what Matlab does, it seems.

src/dense.jl Outdated Show resolved Hide resolved
@martinholters
Copy link
Member Author

Agree, restricting to square A is reasonable.

@stevengj
Copy link
Member

stevengj commented Jan 15, 2025

Would be good to have a @test_throws to ensure it throws for non-square empty matrices.

@martinholters martinholters changed the title Let cond of an empty matrix return zero Let cond of an empty square matrix return zero Jan 21, 2025
Copy link

codecov bot commented Jan 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.89%. Comparing base (57e9a0d) to head (507cb3b).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1169   +/-   ##
=======================================
  Coverage   91.89%   91.89%           
=======================================
  Files          34       34           
  Lines       15355    15358    +3     
=======================================
+ Hits        14110    14113    +3     
  Misses       1245     1245           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

test/dense.jl Outdated Show resolved Hide resolved
Co-authored-by: Steven G. Johnson <[email protected]>
test/dense.jl Outdated Show resolved Hide resolved
@dkarrasch dkarrasch merged commit 1a0135a into master Jan 29, 2025
4 checks passed
@dkarrasch dkarrasch deleted the mh/fix-778 branch January 29, 2025 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cond fails for null dimensions
3 participants